-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Option to track build configuration for changes between builds #34713
Conversation
@snazy we should look into adding a similar feature to our Gradle plugin too. |
You mean to just collect the properties? |
* Whether to use a {@code ~} as an alias for user home directory in path values | ||
*/ | ||
@WithDefault("true") | ||
boolean useUserHomeAliasInPaths(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems out of place.
- why have this as an option in this specific corner of Quarkus ?
- why do we need to refer to user home path anyways for this - is it not supposed to be relative to the project ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah sorry I read it as this was used for reading variables.
..so I grok the relevance but is there even a value in storing this as afaik we don't interpret ~ into ${user.home} when reading the values ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tried using ~
in config values, tbh. If we don't do this kind of transformation, every config will be different for each user. It will often still be even with ~
but user home dir path looks like an obvious thing to ignore in comparisons.
* and their values that are read during the build. | ||
*/ | ||
@Priority(Priorities.APPLICATION) | ||
public class ConfigTrackingInterceptor implements ConfigSourceInterceptor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we split the writing part from the interceptor? The interceptor could just record the values in a static Map, that could be accessed from anywhere.
The writer's logic could then be extracted and applied when required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it could be done better. I just didn't want to expose the interceptor itself with its public API to build steps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made ConfigTrackingInterceptor
expose ConfigTrackingInterceptor.ReadOptionsProvider
that returns a Map<String, String>
of the read options. And the writing part is extracted to ConfigTrackingWriter
.
The reason for using Map<String, String>
instead of Map<String, ConfigValue>
is to be able to record null
config values in a ConcurrentHashMap
@snazy the idea is to collect all config options that were read during the build and persist the report somewhere. Basically, this change already accomplishes this part already for both Maven and Gradle. |
That alone is not going to be enough to make a conclusion about skipping the build, it's going to cover only the config part of the input though. |
622d0f1
to
f096409
Compare
Added a doc and an option to hash (sha-512) config values for certain options. |
f096409
to
7be4d69
Compare
Ready for review. |
@gsmet just fyi, i'd like to backport it to 3.2 |
🙈 The PR is closed and the preview is expired. |
This comment has been minimized.
This comment has been minimized.
// rebuild and compare the files | ||
result = running.execute(List.of("package -DskipTests"), Map.of()); | ||
assertThat(result.getProcess().waitFor()).isEqualTo(0); | ||
assertThat(configDump).exists(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: .exists()
is superfluous (the check 2 lines below does this implicitly)
try (BufferedReader reader = Files.newBufferedReader(configDump.toPath())) { | ||
props.load(reader); | ||
} | ||
assertThat(props.getProperty("quarkus.application.name")).isEqualTo(HashUtil.sha512("code-with-quarkus")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I find it more useful to write something like assertThat(props).containsEntry(key, value)
, because that dumps the whole properties object if the assertion fails, which can be useful.
The property-key checks can be appended like:
assertThat(props)
.containsEntry(key, value)
.extractingFromEntries(e -> e.getKey().toString())
.noneMatch(key -> "quarkus.platform.group-id".equals(key))
.noneMatch(key -> key.startsWith("quarkus.test."));
(Not worth to change though).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
||
The `prod` part of the `quarkus-prod-config-dump` file name refers to the Quarkus build mode, indicating that the dump was taken for the production build. | ||
|
||
The reason `${project.basedir}/.quarkus` directory was chosen as the default location was to make it easy to track build time configuration changes between builds and use that as an indicator to build output caching tools (such as https://maven.apache.org/extensions/maven-build-cache-extension/[Apache Maven Build Cache] and https://gradle.com/gradle-enterprise-solutions/build-cache/[Gradle Enterprise Build Cache]) whether the application binary has to be re-built. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting: for example for the fast-jar packaging, the Gradle plugin caches only the generated parts, excluding all the dependencies. It's not nice, but also not such a big deal to cache dependencies locally, but remote caches suffer.
Don't know how the Maven-Caching extension or Gradle Enterprise's Maven Cache work, so this is just a question: Is it necessary to include a (hash of) the dependencies/dependency-coordinates including their transitive dependencies (if it's not a release version or another project)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if that answers your question but the current implementation of the Maven extension is adding the compilation classpath as input to the Quarkus build
goal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As @jprinet wrote, for now this is based on the assumption that other inputs (dependencies included) are checked by the build outcome caching tools.
8f67e8d
to
1cf2578
Compare
This comment has been minimized.
This comment has been minimized.
Maven goal that compares the recorded build config from the previous build to the current config before building an application Co-authored-by: Robert Stupp <[email protected]>
1cf2578
to
01eedec
Compare
@radcortez would you mind giving it another look? It'd be great to have it backported to 3.2. Thanks. |
This comment has been minimized.
This comment has been minimized.
builder.withInterceptors(buildConfigTracker); | ||
var config = builder.build(); | ||
buildConfigTracker.configure(config); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you need to configure the interceptor, you can use a ConfigSourceInterceptorFactory
instead. This provides you access to the context with the current config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out. Given that I'd need to obtain the reference to the created interceptor, not sure whether it will end up being more elegant at the end though.
Sure. Looks ok to me. Left a small comment, but not a big deal. You can leave it as is and I can change it at some point. |
This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [flow-bin](https://github.com/flowtype/flow-bin) ([changelog](https://github.com/facebook/flow/blob/master/Changelog.md)) | devDependencies | minor | [`^0.214.0` -> `^0.215.0`](https://renovatebot.com/diffs/npm/flow-bin/0.214.0/0.215.1) | | [org.liquibase:liquibase-maven-plugin](http://www.liquibase.org/liquibase-maven-plugin) ([source](https://github.com/liquibase/liquibase)) | build | patch | `4.23.0` -> `4.23.1` | | [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.2.3.Final` -> `3.3.0` | | [io.quarkus:quarkus-universe-bom](https://github.com/quarkusio/quarkus-platform) | import | minor | `3.2.3.Final` -> `3.3.0` | | [org.apache.maven.plugins:maven-enforcer-plugin](https://maven.apache.org/enforcer/) | build | minor | `3.3.0` -> `3.4.0` | --- ### Release Notes <details> <summary>flowtype/flow-bin</summary> ### [`v0.215.1`](flow/flow-bin@a92ce80...cbb038f) [Compare Source](flow/flow-bin@a92ce80...cbb038f) ### [`v0.215.0`](flow/flow-bin@ca11e28...a92ce80) [Compare Source](flow/flow-bin@ca11e28...a92ce80) </details> <details> <summary>liquibase/liquibase</summary> ### [`v4.23.1`](https://github.com/liquibase/liquibase/blob/HEAD/changelog.txt#Liquibase-4231-is-a-patch-release) [Compare Source](liquibase/liquibase@v4.23.0...v4.23.1) </details> <details> <summary>quarkusio/quarkus</summary> ### [`v3.3.0`](https://github.com/quarkusio/quarkus/releases/tag/3.3.0) [Compare Source](quarkusio/quarkus@3.2.4.Final...3.3.0) ##### Complete changelog - [#​35350](quarkusio/quarkus#35350) - Fix package type system property clearing - [#​35348](quarkusio/quarkus#35348) - quarkus-maven-plugin runs native building even if the profile is commented out - [#​35343](quarkusio/quarkus#35343) - ArC: fix StackOverflowError in AutoAddScopeBuildItem - [#​35319](quarkusio/quarkus#35319) - Register arrays of Hibernate ORM's JDBC basic types for reflection - [#​35315](quarkusio/quarkus#35315) - Fix Datasource timing issues with Liquibase / Flyway and OpenTelemetry - [#​35314](quarkusio/quarkus#35314) - Regression in 3.3.0.CR1: Synthetic bean instance for io.opentelemetry.api.OpenTelemetry not initialized yet - [#​35312](quarkusio/quarkus#35312) - Updates Infinispan to 14.0.13.Final - [#​35308](quarkusio/quarkus#35308) - Lock jib execution to avoid OverlappingFileLockException in parallel builds - [#​35305](quarkusio/quarkus#35305) - Fix the titles of the tables in RESTEasy Reactive doc - [#​35302](quarkusio/quarkus#35302) - Docs: Mention wilcard support in resteasy reactive XML serialisation exclude classes configuration - [#​35301](quarkusio/quarkus#35301) - Fix potential NPE in quarkus-csrf-reactive when no MediaType is found - [#​35299](quarkusio/quarkus#35299) - Output build graph using `quarkus.builder.graph-output` property - [#​35285](quarkusio/quarkus#35285) - NullPointerException during http post request when quarkus-csrf-reactive extension is added to a project - [#​35283](quarkusio/quarkus#35283) - Upgrade proto-google-common-protos to 2.23.0 - [#​35282](quarkusio/quarkus#35282) - Avoid keeping references to BytecodeRecorderImpl - [#​35276](quarkusio/quarkus#35276) - Reinstate DevModeTestUtil to avoid breaking other projects that depend on it - [#​35273](quarkusio/quarkus#35273) - Fix small typo in comment - [#​35263](quarkusio/quarkus#35263) - Stop the recovery service while ArC is still around - [#​35245](quarkusio/quarkus#35245) - Add missing info to init Jobs - [#​35244](quarkusio/quarkus#35244) - Init Jobs are missing ServiceAccount and Image Pull Secrets - [#​35240](quarkusio/quarkus#35240) - Update SmallRye Health to 4.0.4 - [#​34071](quarkusio/quarkus#34071) - 3.1.1 Final - java.lang.IllegalArgumentException: Class java.util.UUID\[] is instantiated reflectively but was never registered - [#​32800](quarkusio/quarkus#32800) - Duplicated checks in health check response - [#​11903](quarkusio/quarkus#11903) - Gradle multimodule project + quarkus-container-image-jib: OverlappingFileLockException ### [`v3.2.4.Final`](https://github.com/quarkusio/quarkus/releases/tag/3.2.4.Final) [Compare Source](quarkusio/quarkus@3.2.3.Final...3.2.4.Final) ##### Complete changelog - [#​35300](quarkusio/quarkus#35300) - Fix `jandex-gradle-plugin-version` in CDI Reference - [#​35296](quarkusio/quarkus#35296) - Upgrade H2 to 2.2.220 - [#​35258](quarkusio/quarkus#35258) - CDI Reference 1.1 has incomplete information for gradle - [#​35255](quarkusio/quarkus#35255) - Quartz: QuarkusMSSQLDelegate should extends MSSQLDelegate - [#​35211](quarkusio/quarkus#35211) - Document Maven config options that may be relevant when running tests - [#​35203](quarkusio/quarkus#35203) - Pass Maven user settings when initializing artifact resolver - [#​35193](quarkusio/quarkus#35193) - OpenTelemetry service name should have higher priority than app name when no resource attributes are set - [#​35189](quarkusio/quarkus#35189) - Quarkus CLI fixes - [#​35188](quarkusio/quarkus#35188) - SmallRyeGraphQLOverWebSocketHandler: use order value > Integer.MIN_VALUE - [#​35181](quarkusio/quarkus#35181) - REST Data with Panache should not produce links when hal is disabled - [#​35174](quarkusio/quarkus#35174) - Ensure the narayana-jta extension fully shuts down the recovery manager - [#​35172](quarkusio/quarkus#35172) - Kafka Streams: restore the feature name at Quarkus startup - [#​35171](quarkusio/quarkus#35171) - kafka-streams: feature not listed on startup - [#​35165](quarkusio/quarkus#35165) - Propagate all user methods in REST Data with Panache - [#​35160](quarkusio/quarkus#35160) - Properly use internal links to point to other guides - [#​35140](quarkusio/quarkus#35140) - ArC: fix deadlock when calling guarded methods on the same thread - [#​35136](quarkusio/quarkus#35136) - Deadlock while calling write-locked method from read-locked method - [#​34908](quarkusio/quarkus#34908) - `@RouteFilter` stopped working with WebSocket requests Quarkus 3.2.0.Final - [#​34875](quarkusio/quarkus#34875) - Quarkus build does not work since 3.2.0 with teamcity/plexus launcher - [#​34713](quarkusio/quarkus#34713) - Option to track build configuration for changes between builds - [#​34576](quarkusio/quarkus#34576) - Live reload stopped working on 3.2 when using XA transactions - [#​34514](quarkusio/quarkus#34514) - Support `@WithUnnamedKey` in documentation - [#​34065](quarkusio/quarkus#34065) - Add support for project Java version update based on extensions - [#​33317](quarkusio/quarkus#33317) - OpenTelemetry SDK autoconfiguration ignores OTEL service name in favor of Quarkus app name - [#​15461](quarkusio/quarkus#15461) - Quarkus tests fails mTLS authentication against internal Maven repository </details> <details> <summary>quarkusio/quarkus-platform</summary> ### [`v3.3.0`](quarkusio/quarkus-platform@3.2.4.Final...3.3.0) [Compare Source](quarkusio/quarkus-platform@3.2.4.Final...3.3.0) ### [`v3.2.4.Final`](quarkusio/quarkus-platform@3.2.3.Final...3.2.4.Final) [Compare Source](quarkusio/quarkus-platform@3.2.3.Final...3.2.4.Final) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [x] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
…s-googlecloud-jsonlogging!17) This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [io.quarkus:quarkus-extension-processor](https://github.com/quarkusio/quarkus) | | minor | `3.2.3.Final` -> `3.3.0` | | [io.quarkus:quarkus-extension-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.2.3.Final` -> `3.3.0` | | [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | minor | `3.2.3.Final` -> `3.3.0` | | [io.quarkus:quarkus-bom](https://github.com/quarkusio/quarkus) | import | minor | `3.2.3.Final` -> `3.3.0` | --- ### Release Notes <details> <summary>quarkusio/quarkus</summary> ### [`v3.3.0`](quarkusio/quarkus@3.2.4.Final...3.3.0) [Compare Source](quarkusio/quarkus@3.2.4.Final...3.3.0) ### [`v3.2.4.Final`](https://github.com/quarkusio/quarkus/releases/tag/3.2.4.Final) [Compare Source](quarkusio/quarkus@3.2.3.Final...3.2.4.Final) ##### Complete changelog - [#​35300](quarkusio/quarkus#35300) - Fix `jandex-gradle-plugin-version` in CDI Reference - [#​35296](quarkusio/quarkus#35296) - Upgrade H2 to 2.2.220 - [#​35258](quarkusio/quarkus#35258) - CDI Reference 1.1 has incomplete information for gradle - [#​35255](quarkusio/quarkus#35255) - Quartz: QuarkusMSSQLDelegate should extends MSSQLDelegate - [#​35211](quarkusio/quarkus#35211) - Document Maven config options that may be relevant when running tests - [#​35203](quarkusio/quarkus#35203) - Pass Maven user settings when initializing artifact resolver - [#​35193](quarkusio/quarkus#35193) - OpenTelemetry service name should have higher priority than app name when no resource attributes are set - [#​35189](quarkusio/quarkus#35189) - Quarkus CLI fixes - [#​35188](quarkusio/quarkus#35188) - SmallRyeGraphQLOverWebSocketHandler: use order value > Integer.MIN_VALUE - [#​35181](quarkusio/quarkus#35181) - REST Data with Panache should not produce links when hal is disabled - [#​35174](quarkusio/quarkus#35174) - Ensure the narayana-jta extension fully shuts down the recovery manager - [#​35172](quarkusio/quarkus#35172) - Kafka Streams: restore the feature name at Quarkus startup - [#​35171](quarkusio/quarkus#35171) - kafka-streams: feature not listed on startup - [#​35165](quarkusio/quarkus#35165) - Propagate all user methods in REST Data with Panache - [#​35160](quarkusio/quarkus#35160) - Properly use internal links to point to other guides - [#​35140](quarkusio/quarkus#35140) - ArC: fix deadlock when calling guarded methods on the same thread - [#​35136](quarkusio/quarkus#35136) - Deadlock while calling write-locked method from read-locked method - [#​34908](quarkusio/quarkus#34908) - `@RouteFilter` stopped working with WebSocket requests Quarkus 3.2.0.Final - [#​34875](quarkusio/quarkus#34875) - Quarkus build does not work since 3.2.0 with teamcity/plexus launcher - [#​34713](quarkusio/quarkus#34713) - Option to track build configuration for changes between builds - [#​34576](quarkusio/quarkus#34576) - Live reload stopped working on 3.2 when using XA transactions - [#​34514](quarkusio/quarkus#34514) - Support `@WithUnnamedKey` in documentation - [#​34065](quarkusio/quarkus#34065) - Add support for project Java version update based on extensions - [#​33317](quarkusio/quarkus#33317) - OpenTelemetry SDK autoconfiguration ignores OTEL service name in favor of Quarkus app name - [#​15461](quarkusio/quarkus#15461) - Quarkus tests fails mTLS authentication against internal Maven repository </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
An option to record and dump all used build time config options and a Maven goal that compares the recorded build config from the previous build to the current config before building an application.
This is useful for tools that enable Maven goal outcome caching that by tracking changes in the inputs of those goals.
An example application enabling build config tracking is here https://github.com/aloubyansky/playground/tree/dump-config-app-poc
Config tracking is enabled with
quarkus.config-tracking.enabled=true
and is persisted in${project.dir}/.quarkus/quarkus-prod-config-dump
.Other config options currently include:
quarkus.config-tracking.exclude
- a list of option names that should not be tracked (name patterns are supported);quarkus.config-tracking.use-user-home-alias-in-paths
- whether to replace user home dir path in path values with~
(enabled by default)Todo:
FYI @jprinet